Move blocking readPair call to boundedElastic thread #1653
Conversation
| .doOnNext(counters -> readRepair(mailbox, counters)); | ||
| .doOnNext(counters -> { | ||
| Mono.fromRunnable(() -> { | ||
| readRepair(mailbox, counters); |
There was a problem hiding this comment.
Can we manage to subscribe here only if read repairs are enabled ie probability > 0 ?
There was a problem hiding this comment.
Sure, you mean something like:
.doOnNext(counters -> {
if (probability > 0) {
Mono.fromRunnable(() -> {
readRepair(mailbox, counters);
})
.subscribeOn(Schedulers.boundedElastic())
.subscribe();
}
});
There was a problem hiding this comment.
@chibenwa I'm sorry could you guide how to access probability property here? 🤔
Arooba-git
force-pushed
the
fix-blocking-call-in-mailbox
branch
from
d2c426a to
dec0b5d
Compare
| @@ -170,7 +170,16 @@ public Mono<MailboxCounters> getMailboxCountersReactive(Mailbox mailbox) { | |||
| } | |||
| return Mono.just(counters); | |||
| }) | |||
| .doOnNext(counters -> readRepair(mailbox, counters)); | |||
| .doOnNext(counters -> { | |||
| if (this.cassandraConfiguration.getMailboxReadRepair() > 0) { | |||
There was a problem hiding this comment.
I'm oretty sure it should be
| if (this.cassandraConfiguration.getMailboxReadRepair() > 0) { | |
| if (this.cassandraConfiguration.getMailboxCountersReadRepair() > 0) { |
There was a problem hiding this comment.
Oh, you mean we should update the name in the original configuration file too? 🤔
There was a problem hiding this comment.
I believe what chibenwa is saying is:
getMailboxReadRepair()is used inCassandraMailboxMapper- for
CassandraMessageMapper, we usegetMailboxCountersReadRepairChanceMax()and/orgetMailboxCountersReadRepairChanceOneHundred()
There was a problem hiding this comment.
Maybe we could extract that line actually in a function to avoid duplication, that we could call from there and from your code change :)
|
Also read repair could be a good place for pseudo random usage aka non blocking... There is no need for cryptographioc grade randomness for such an application. Maybe otherall this could be a better fix. WDYT? |
Hi! 🙂
We noticed you did a great job in ensuring the reactive modules indeed stay reactive end to end. The mailbox module, however, was discovered to still have a blocking call in
CassandraMessageMapperclass as detected by BlockHound:This PR fixes this code. We also re-ran the tests and verified the performance (in terms of heap usage) before and after the fix:
Before
After